-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
jvm.gc.live.data.size
and max
not updating for optavgpause/optthruput collectors
#2874
jvm.gc.live.data.size
and max
not updating for optavgpause/optthruput collectors
#2874
Conversation
System.gc(); | ||
await().timeout(200, TimeUnit.MILLISECONDS).alias("NotificationListener takes time after GC") | ||
.untilAsserted(() -> | ||
assertThat(registry.find("jvm.gc.live.data.size").gauge().value()).isPositive()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By forcing a GC and waiting for the notification listener to update metrics, this test does a much better job of ensuring expected metrics are available for the configured collector.
await().timeout(200, TimeUnit.MILLISECONDS).alias("NotificationListener takes time after GC") | ||
.untilAsserted(() -> | ||
assertThat(registry.find("jvm.gc.live.data.size").gauge().value()).isPositive()); | ||
assertThat(registry.find("jvm.gc.memory.allocated").counter().count()).isPositive(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This metric is still missing for optavgpause
and optthruput
due to the allocation pool name not being set. We don't have a CI job run with OpenJ9 because we're still waiting for CI images (possibly CircleCI-Public/cimg-openjdk#78). I think we can fix the missing allocated counter separately from this pull request because it is still an improvement to get rid of the NPE which allows updating of the live and max data size gauges.
With the OpenJ9 non-generational collectors `optavgpause` and `optthruput`, a NPE was happening in the notification listener which caused some code in it to not be executed. Namely, setting the live and max data size was being effectively skipped for these collectors. Now we check for `null` and only the allocated bytes counter is skipped when no allocation pool name is set.
3b6316b
to
305ebd3
Compare
|
||
assumeTrue(binder.isGenerationalGc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the condition failed, the whole test was marked as skipped, but if we want to use this same test for generational and non-generational collectors, we should run the part of this test before here for both and report it as passed (not skipped) for non-generational collectors if all assertions passed before here.
jvm.gc.live.data.size
and max
not updating for optavgpause/optthruput collectors
With the OpenJ9 non-generational collectors
optavgpause
andoptthruput
, a NPE was happening in the notification listener which caused some code in it to not be executed. Namely, setting the live and max data size was being effectively skipped for these collectors.Now we check for
null
and only the allocated bytes counter is skipped when no allocation pool name is set.